Properly handle optional event modes #659
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Both bracketed paste and the kitty keyboard enhancement flag are not always supported. This goes both for terminals that may not emit events according to their spec and also applications that will run in the terminal independent of reedline. Make sure we enter the mode when starting to take control and exit the mode when leaving the mode or being dropped. The settings exposed to the user will just internally enable a setting (in the case of the kitty keyboard enhancement flags crossterm provides and easy way to check for support that we take into account with that)
Codecov Report
@@ Coverage Diff @@
## main #659 +/- ##
==========================================
- Coverage 49.22% 49.19% -0.04%
==========================================
Files 43 46 +3
Lines 7912 7930 +18
==========================================
+ Hits 3895 3901 +6
- Misses 4017 4029 +12
|
|
Nice work @sholderbach!! |
|
are we going to land this before the release or after? |
Public as `reedline::kitty_protocol_available` Replace the method `Reedline::can_use_kitty_protocol` that doesn't depend on `Reedline` in the future Rewrite `KittyProtocolGuard` in terms of it.
As we internally check whether it is available, this should be fine in nearly all terminals (if they can safely be probed)
sholderbach
added a commit
to sholderbach/nushell
that referenced
this pull request
Nov 13, 2023
Go from the ill-defined `enable/disable` pairs to `.use_...` builders This alleviates unclear properties when the underlying enhancements are enabled. Now they are enabed when entering `Reedline::read_line` and disabled when exiting that. Furthermore allow setting `$env.config.use_kitty_protocol` to have an effect when toggling during runtime. Previously it was only enabled when receiving a value from `config.nu`. I kept the warning code there to not pollute the log. We could move it into the REPL-loop if desired Not sure if we should actively block the enabling of `bracketed_paste` on Windows. Need to test what happens if it just doesn't do anything we could remove the `cfg!` switch. At least for WSL2 Windows Terminal already supports bracketed paste. `target_os = windows` is a bad predictor for `conhost.exe`. Depends on nushell/reedline#659 (pointing to personal fork) Closes nushell#10982 Supersedes nushell#10998
sholderbach
added a commit
to nushell/nushell
that referenced
this pull request
Nov 14, 2023
Go from the ill-defined `enable/disable` pairs to `.use_...` builders This alleviates unclear properties when the underlying enhancements are enabled. Now they are enabed when entering `Reedline::read_line` and disabled when exiting that. Furthermore allow setting `$env.config.use_kitty_protocol` to have an effect when toggling during runtime. Previously it was only enabled when receiving a value from `config.nu`. I kept the warning code there to not pollute the log. We could move it into the REPL-loop if desired Not sure if we should actively block the enabling of `bracketed_paste` on Windows. Need to test what happens if it just doesn't do anything we could remove the `cfg!` switch. At least for WSL2 Windows Terminal already supports bracketed paste. `target_os = windows` is a bad predictor for `conhost.exe`. Depends on nushell/reedline#659 (pointing to personal fork) Closes #10982 Supersedes #10998
hardfau1t
pushed a commit
to hardfau1t/nushell
that referenced
this pull request
Dec 14, 2023
Go from the ill-defined `enable/disable` pairs to `.use_...` builders This alleviates unclear properties when the underlying enhancements are enabled. Now they are enabed when entering `Reedline::read_line` and disabled when exiting that. Furthermore allow setting `$env.config.use_kitty_protocol` to have an effect when toggling during runtime. Previously it was only enabled when receiving a value from `config.nu`. I kept the warning code there to not pollute the log. We could move it into the REPL-loop if desired Not sure if we should actively block the enabling of `bracketed_paste` on Windows. Need to test what happens if it just doesn't do anything we could remove the `cfg!` switch. At least for WSL2 Windows Terminal already supports bracketed paste. `target_os = windows` is a bad predictor for `conhost.exe`. Depends on nushell/reedline#659 (pointing to personal fork) Closes nushell#10982 Supersedes nushell#10998
dmatos2012
pushed a commit
to dmatos2012/nushell
that referenced
this pull request
Feb 20, 2024
Go from the ill-defined `enable/disable` pairs to `.use_...` builders This alleviates unclear properties when the underlying enhancements are enabled. Now they are enabed when entering `Reedline::read_line` and disabled when exiting that. Furthermore allow setting `$env.config.use_kitty_protocol` to have an effect when toggling during runtime. Previously it was only enabled when receiving a value from `config.nu`. I kept the warning code there to not pollute the log. We could move it into the REPL-loop if desired Not sure if we should actively block the enabling of `bracketed_paste` on Windows. Need to test what happens if it just doesn't do anything we could remove the `cfg!` switch. At least for WSL2 Windows Terminal already supports bracketed paste. `target_os = windows` is a bad predictor for `conhost.exe`. Depends on nushell/reedline#659 (pointing to personal fork) Closes nushell#10982 Supersedes nushell#10998
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Both bracketed paste and the kitty keyboard enhancement flag are not always supported.
This goes both for terminals that may not emit events according to the extensions thus we already provide the means to only optionally enable them.
But also the applications that may run between each
Reedline::read_linecall are in no way guaranteed to supported any enhanced terminal events coming in.It should thus be part of reedline's contract to leave
read_linein a clean state with the canonical terminal behavior.The API for
Reedline::enable_bracketed_pastewas misleading as it for the most part acted independent of theReedlinestate and immediately globally enabled bracketed paste, potentially polluting any application during evaluation.disable_bracketed_pasteequally globally modified state but was only active if the bracketed paste state was entered withenable_bracketed_pasteand would only run once.For better readability transfer this pattern to the kitty keyboard enhancement protocol. (which was already properly handled but was overly eager to check if the enhancements are supported on every
read_line)Make sure we enter the mode when starting to take control and exit the mode when leaving the mode or being dropped.
The settings exposed to the user will just internally enable a setting (in the case of the kitty keyboard enhancement flags crossterm provides an easy way to check for support that we take into account with that)
Close nushell/nushell#10982 and related issues for non-nushell projects using reedline
As a follow up we can simplify the setters to ignore the
Resultor collapse it to a simplerfn use_...(self, enable: bool)